fix: update TopicUpdateTransaction to use Key instead of PublicKey #2241
Conversation
…r key parameters Signed-off-by: oGranny <ogranny.github.io@gmail.com>
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Pull request overview
This PR updates TopicUpdateTransaction to accept the generic Key interface (rather than PublicKey) for all key-related fields, aligning topic update behavior with other transactions that already support PrivateKey/PublicKey interchangeably.
Changes:
- Switched
TopicUpdateTransactionkey-related fields and setters fromPublicKeytoKey. - Updated protobuf building to use the shared
key_to_proto()helper for key serialization. - Expanded unit tests to validate both Ed25519/ECDSA and PrivateKey/PublicKey inputs for all relevant topic update key fields.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/hiero_sdk_python/consensus/topic_update_transaction.py |
Migrates topic update key parameters to Key and serializes keys via key_to_proto() in the proto body builder. |
tests/unit/topic_update_transaction_test.py |
Adds parametrized coverage for admin/submit/fee schedule/fee exempt keys using both algorithms and both private/public key inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughTopicUpdateTransaction is refactored to use the generic Key type instead of PublicKey for key-related fields (admin_key, submit_key, fee_schedule_key, fee_exempt_keys). The constructor and setters accept Key types, and protobuf serialization uses key_to_proto conversion. Support for custom fees and related fields is added. ChangesTopicUpdateTransaction Key Type Modernization
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hiero_sdk_python/consensus/topic_update_transaction.py (1)
96-122: 🧹 Nitpick | 🔵 Trivial | 💤 Low valuePolish setter docstrings to match the new
Keycontract.Two small inconsistencies introduced by the refactor:
- The phrase "Sets the public admin/submit key for the topic." (lines 98, 112) is now inaccurate — the parameter is the generic
Keytype and may be aPrivateKey,PublicKey, or any future SDKKeysubclass. Only the public-key bytes are serialized to the wire (viakey_to_proto→to_proto_key()), but the API itself no longer constrains callers to public keys.- The Args entries on lines 101 and 115 are malformed — they list the type in place of the parameter name. Lines 190 and 204 already use the correct
key (Key): …/keys (list[Key]): …form, so these two stand out.📝 Proposed docstring corrections
def set_admin_key(self, key: Key) -> TopicUpdateTransaction: """ - Sets the public admin key for the topic. + Sets the admin key for the topic. Accepts any ``Key`` (e.g. ``PrivateKey`` + or ``PublicKey``); only the public form is serialized on the wire. Args: - Key: The admin key to set. + key (Key): The admin key to set.def set_submit_key(self, key: Key) -> TopicUpdateTransaction: """ - Sets the public submit key for the topic. + Sets the submit key for the topic. Accepts any ``Key`` (e.g. ``PrivateKey`` + or ``PublicKey``); only the public form is serialized on the wire. Args: - Key: The submit key to set. + key (Key): The submit key to set.Also applies to: 185-211
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8ae67766-cd8d-4500-928a-ce4f1a8afec4
📒 Files selected for processing (2)
src/hiero_sdk_python/consensus/topic_update_transaction.pytests/unit/topic_update_transaction_test.py
Signed-off-by: oGranny <ogranny.github.io@gmail.com>
|
Hello, this is the OfficeHourBot. This is a reminder that the Hiero Python SDK Office Hours are scheduled in approximately 4 hours (14:00 UTC). This session provides an opportunity to ask questions regarding this Pull Request. Details:
Disclaimer: This is an automated reminder. Please verify the schedule here for any changes. From, |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #2241 +/- ##
=======================================
Coverage 93.73% 93.73%
=======================================
Files 145 145
Lines 9482 9483 +1
=======================================
+ Hits 8888 8889 +1
Misses 594 594 🚀 New features to boost your workflow:
|
|
Thank you @oGranny ! |
…iero-ledger#2241) Signed-off-by: oGranny <ogranny.github.io@gmail.com>
…iero-ledger#2241) Signed-off-by: oGranny <ogranny.github.io@gmail.com>
Description:
Update the
TopicUpdateTransactionclass to accept the genericKeyinterface for all key-related fields instead ofPublicKeyRelated issue(s):
Fixes #2214
Checklist